-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add filtering options to rustc_on_unimplemented
#47613
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
r? @Kimundi (rust_highfive has picked a reviewer for you, use r? to override) |
d47b4df
to
ffdcd35
Compare
src/libsyntax/parse/parser.rs
Outdated
} | ||
|
||
fn parse_ident_common(&mut self, recover: bool) -> PResult<'a, ast::Ident> { | ||
pub fn parse_ident_attr(&mut self) -> PResult<'a, ast::Ident> { | ||
self.parse_ident_common(true, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add the logic for Self
here in parse_ident_attr
instead of adding one more tweak to parse_ident_common
.
(See parse_path_segment_ident
for an example.)
c42bcb7
to
9339450
Compare
Ping. |
@nikomatsakis just as a sanity check: currently #[rustc_on_unimplemented(
on(
Self="&str",
label="`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"
),
label="`{Self}` is not an iterator; maybe try calling `.iter()` or a similar method"
)] Is this acceptable? Otherwise we can just keep what this PR is currently using ( |
I .. think this is true, but I don't quite know what "anywhere else" means. Do you mean that you can't define your own type parameters named |
If so, I believe that is true |
|
Also, when adding a parser change, how long do we have to wait until we can start using that change into |
Ah, I see. Hmm, that makes me a mite uncomfortable.
You may have to wait until the next beta, I suppose, so that the code can be parsed with stage0. Sometimes a |
OK, so, I feel a bit weird about singling out #[foo(
let = 3,
static = 4,
)] than just I sort of expect to replace Thoughts @rust-lang/lang / @petrochenkov on whether it's worrisome to allow identifiers inside of attributes like that? |
I don't think If we decide not to start accepting kws in attributes, the workaround used here ( |
If we're blocked on a decision regarding allowing |
@estebank I think i'd .. prefer that for now. This is kind of an insta-stable change I'd rather we approach carefully. That said, I think we should probably permit all attributes. |
@nikomatsakis done. |
src/libsyntax/parse/parser.rs
Outdated
pub fn parse_ident(&mut self) -> PResult<'a, ast::Ident> { | ||
self.parse_ident_common(true) | ||
} | ||
|
||
pub fn parse_ident_attr(&mut self) -> PResult<'a, ast::Ident> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: now this function is unnecessary.
- filter error on the evaluated value of `Self` - filter error on the evaluated value of the type arguments - add argument to include custom note in diagnostic - allow the parser to parse `Self` when processing attributes - add custom message to binops
@nikomatsakis ping from triage! Could you review this? |
@bors r+ |
📌 Commit fd3f231 has been approved by |
⌛ Testing commit fd3f231 with merge 51b336dc90cf0fe21beeb9184ab856985da96d79... |
💔 Test failed - status-appveyor |
⌛ Testing commit fd3f231 with merge a869c5e36cec50960ee1d9b482f38944c9dce01d... |
💔 Test failed - status-travis |
Looks like a new kind of spurious error introduced by making trans dynamic. cc @alexcrichton @bjorn3
@bors retry |
@kennytm hm that may be https://sourceware.org/bugzilla/show_bug.cgi?id=20608 and may indicate we need to update binutils on that bot... Let's see if it happens again? |
…nikomatsakis Add filtering options to `rustc_on_unimplemented` - Add filtering options to `rustc_on_unimplemented` for local traits, filtering on `Self` and type arguments. - Add a way to provide custom notes. - Tweak binops text. - Add filter to detect wether `Self` is local or belongs to another crate. - Add filter to `Iterator` diagnostic for `&str`. Partly addresses rust-lang#44755 with a different syntax, as a first approach. Fixes rust-lang#46216, fixes rust-lang#37522, CC rust-lang#34297, rust-lang#46806.
rustc_on_unimplemented
for local traits, filtering onSelf
and type arguments.Self
is local or belongs to another crate.Iterator
diagnostic for&str
.Partly addresses #44755 with a different syntax, as a first approach. Fixes #46216, fixes #37522, CC #34297, #46806.